Skip to content

Conversation

@aditi-pandit
Copy link
Contributor

@aditi-pandit aditi-pandit commented Oct 21, 2025

== NO RELEASE NOTE ==

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Oct 21, 2025
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Oct 21, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR advances the Velox submodule to a newer commit, updates the native execution build configuration, and refactors code to accommodate API changes introduced by the upgraded Velox version.

File-Level Changes

Change Details Files
Advance Velox submodule to latest commit
  • Updated git submodule pointer to new Velox SHA
  • Bumped version tag in submodule configuration
presto-native-execution/velox
Update build configuration for new Velox layout
  • Adjusted CMakeLists.txt to reflect updated include paths
  • Revised BUILD targets to incorporate relocated libraries
presto-native-execution/velox/CMakeLists.txt
presto-native-execution/velox/BUILD
Refactor native execution code for API compatibility
  • Replaced deprecated Velox API calls with new equivalents
  • Updated function signatures and data conversion logic
  • Aligned tests with updated API behavior
presto-native-execution/velox/src

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@aditi-pandit aditi-pandit changed the title chore(native): Advance Velox [Do not review]chore(native): Advance Velox Oct 21, 2025
@aditi-pandit aditi-pandit force-pushed the advance_velox branch 2 times, most recently from e22806d to 4fa054c Compare October 24, 2025 01:02
@aditi-pandit aditi-pandit marked this pull request as ready for review October 31, 2025 07:41
@aditi-pandit aditi-pandit requested review from a team as code owners October 31, 2025 07:41
@prestodb-ci prestodb-ci requested review from a team, namya28 and sh-shamsan and removed request for a team October 31, 2025 07:41
@aditi-pandit aditi-pandit changed the title [Do not review]chore(native): Advance Velox chore(ci): Advance Velox Oct 31, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@aditi-pandit
Copy link
Contributor Author

aditi-pandit commented Oct 31, 2025

Error for macos build :

ld: Undefined symbols:
  _AbslInternalSpinLockWake_lts_20250814, referenced from:
      absl::lts_20250814::base_internal::SpinLockWake(std::__1::atomic<unsigned int>*, bool) in libvelox_functions_prestosql.a[10](DateTimeFunctionsRegistration.cpp.o)
  absl::lts_20250814::base_internal::SpinLockWait(std::__1::atomic<unsigned int>*, int, absl::lts_20250814::base_internal::SpinLockWaitTransition const*, absl::lts_20250814::base_internal::SchedulingMode), referenced from:
      void absl::lts_20250814::base_internal::CallOnceImpl<void (*)(re2::LazyRE2 const*), re2::LazyRE2 const*>(std::__1::atomic<unsigned int>*, absl::lts_20250814::base_internal::SchedulingMode, void (*&&)(re2::LazyRE2 const*), re2::LazyRE2 const*&&) in libvelox_functions_prestosql.a[10](DateTimeFunctionsRegistration.cpp.o)
  absl::lts_20250814::raw_log_internal::RawLog(absl::lts_20250814::LogSeverity, char const*, int, char const*, ...), referenced from:
      void absl::lts_20250814::base_internal::CallOnceImpl<void (*)(re2::LazyRE2 const*), re2::LazyRE2 const*>(std::__1::atomic<unsigned int>*, absl::lts_20250814::base_internal::SchedulingMode, void (*&&)(re2::LazyRE2 const*), re2::LazyRE2 const*&&) in libvelox_functions_prestosql.a[10](DateTimeFunctionsRegistration.cpp.o)
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@czentgr @majetideepak : I am seeing this error for this job only on this build. The history for it is green as well. Can't pinpoint from the Velox changes what might have caused this. Any suggestions ?

@aditi-pandit
Copy link
Contributor Author

aditi-pandit commented Oct 31, 2025

Arrow flight tests error

velox/velox/functions/prestosql/registration/CMakeFiles/velox_functions_prestosql.dir/DateTimeFunctionsRegistration.cpp.o -MF velox/velox/functions/prestosql/registration/CMakeFiles/velox_functions_prestosql.dir/DateTimeFunctionsRegistration.cpp.o.d -o velox/velox/functions/prestosql/registration/CMakeFiles/velox_functions_prestosql.dir/DateTimeFunctionsRegistration.cpp.o -c /__w/presto/presto/presto-native-execution/velox/velox/functions/prestosql/registration/DateTimeFunctionsRegistration.cpp
In file included from /__w/presto/presto/presto-native-execution/velox/velox/functions/prestosql/registration/DateTimeFunctionsRegistration.cpp:18:
/__w/presto/presto/presto-native-execution/velox/velox/functions/prestosql/DateTimeFunctions.h:19:10: fatal error: fast_float/fast_float.h: No such file or directory
   19 | #include <fast_float/fast_float.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

That comes from facebookincubator/velox@909429d

The full command line is

ccache /opt/rh/gcc-toolset-12/root/bin/g++ -DBOOST_ATOMIC_DYN_LINK -DBOOST_ATOMIC_NO_LIB -DBOOST_CONTEXT_DYN_LINK -DBOOST_CONTEXT_NO_LIB -DBOOST_FILESYSTEM_DYN_LINK -DBOOST_FILESYSTEM_NO_LIB -DBOOST_PROGRAM_OPTIONS_DYN_LINK -DBOOST_PROGRAM_OPTIONS_NO_LIB -DBOOST_REGEX_DYN_LINK -DBOOST_REGEX_NO_LIB -DBOOST_SYSTEM_DYN_LINK -DBOOST_SYSTEM_NO_LIB -DBOOST_THREAD_DYN_LINK -DBOOST_THREAD_NO_LIB -DFOLLY_CFG_NO_COROUTINES -DFOLLY_HAVE_INT128_T=1 -DGEOS_INLINE -DGFLAGS_IS_A_DLL=0 -DNDEBUG -DSIMDJSON_THREADS_ENABLED=1 -DUSE_UNSTABLE_GEOS_CPP_API -DVELOX_ENABLE_GEO -I/__w/presto/presto/adapter-deps/install/include -I/__w/presto/presto/presto-native-execution/. -I/__w/presto/presto/presto-native-execution/velox -I/__w/presto/presto/presto-native-execution/velox/velox/external/xxhash -I/__w/presto/presto/presto-native-execution/_build/release/velox -I/__w/presto/presto/presto-native-execution/_build/release -I/__w/presto/presto/presto-native-execution/velox/. -I/__w/presto/presto/presto-native-execution/_build/release/_deps/xsimd-src/include -I/__w/presto/presto/presto-native-execution/_build/release/_deps/simdjson-src/include -isystem /__w/presto/presto/presto-native-execution/velox/velox -isystem /__w/presto/presto/presto-native-execution/velox/velox/external -isystem /usr/include/libdwarf-0 -isystem /__w/presto/presto/presto-native-execution/_build/release/_deps/geos-src/include -isystem /__w/presto/presto/presto-native-execution/_build/release/_deps/geos-build/include -isystem /__w/presto/presto/presto-native-execution/_build/release/_deps/geos-src/src/deps -isystem /__w/presto/presto/presto-native-execution/_build/release/_deps/geos-build/src/deps -mavx2 -mfma -mavx -mf16c -mlzcnt -mbmi2 -Wno-nullability-completeness -Wno-deprecated-declarations -Wreorder -mavx2 -mfma -mavx -mf16c -mlzcnt -mbmi2 -D USE_VELOX_COMMON_BASE -D HAS_UNCAUGHT_EXCEPTIONS -DFOLLY_CFG_NO_COROUTINES -Wall -Wextra -Wno-unused -Wno-unused-parameter -Wno-sign-compare -Wno-ignored-qualifiers -Wno-implicit-fallthrough -Wno-class-memaccess -Wno-comment -Wno-int-in-bool-context -Wno-redundant-move -Wno-array-bounds -Wno-maybe-uninitialized -Wno-unused-result -Wno-format-overflow -Wno-strict-aliasing -Wno-restrict -Werror -O3 -DNDEBUG -std=gnu++20 -fPIC -Wno-deprecated-declarations -ffp-contract=off -MD -MT

@czentgr, @majetideepak : Is this because of ccache use ?

@MBkkt

@aditi-pandit
Copy link
Contributor Author

Arrow flight tests error

velox/velox/functions/prestosql/registration/CMakeFiles/velox_functions_prestosql.dir/DateTimeFunctionsRegistration.cpp.o -MF velox/velox/functions/prestosql/registration/CMakeFiles/velox_functions_prestosql.dir/DateTimeFunctionsRegistration.cpp.o.d -o velox/velox/functions/prestosql/registration/CMakeFiles/velox_functions_prestosql.dir/DateTimeFunctionsRegistration.cpp.o -c /__w/presto/presto/presto-native-execution/velox/velox/functions/prestosql/registration/DateTimeFunctionsRegistration.cpp
In file included from /__w/presto/presto/presto-native-execution/velox/velox/functions/prestosql/registration/DateTimeFunctionsRegistration.cpp:18:
/__w/presto/presto/presto-native-execution/velox/velox/functions/prestosql/DateTimeFunctions.h:19:10: fatal error: fast_float/fast_float.h: No such file or directory
   19 | #include <fast_float/fast_float.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

That comes from facebookincubator/velox@909429d

The full command line is

ccache /opt/rh/gcc-toolset-12/root/bin/g++ -DBOOST_ATOMIC_DYN_LINK -DBOOST_ATOMIC_NO_LIB -DBOOST_CONTEXT_DYN_LINK -DBOOST_CONTEXT_NO_LIB -DBOOST_FILESYSTEM_DYN_LINK -DBOOST_FILESYSTEM_NO_LIB -DBOOST_PROGRAM_OPTIONS_DYN_LINK -DBOOST_PROGRAM_OPTIONS_NO_LIB -DBOOST_REGEX_DYN_LINK -DBOOST_REGEX_NO_LIB -DBOOST_SYSTEM_DYN_LINK -DBOOST_SYSTEM_NO_LIB -DBOOST_THREAD_DYN_LINK -DBOOST_THREAD_NO_LIB -DFOLLY_CFG_NO_COROUTINES -DFOLLY_HAVE_INT128_T=1 -DGEOS_INLINE -DGFLAGS_IS_A_DLL=0 -DNDEBUG -DSIMDJSON_THREADS_ENABLED=1 -DUSE_UNSTABLE_GEOS_CPP_API -DVELOX_ENABLE_GEO -I/__w/presto/presto/adapter-deps/install/include -I/__w/presto/presto/presto-native-execution/. -I/__w/presto/presto/presto-native-execution/velox -I/__w/presto/presto/presto-native-execution/velox/velox/external/xxhash -I/__w/presto/presto/presto-native-execution/_build/release/velox -I/__w/presto/presto/presto-native-execution/_build/release -I/__w/presto/presto/presto-native-execution/velox/. -I/__w/presto/presto/presto-native-execution/_build/release/_deps/xsimd-src/include -I/__w/presto/presto/presto-native-execution/_build/release/_deps/simdjson-src/include -isystem /__w/presto/presto/presto-native-execution/velox/velox -isystem /__w/presto/presto/presto-native-execution/velox/velox/external -isystem /usr/include/libdwarf-0 -isystem /__w/presto/presto/presto-native-execution/_build/release/_deps/geos-src/include -isystem /__w/presto/presto/presto-native-execution/_build/release/_deps/geos-build/include -isystem /__w/presto/presto/presto-native-execution/_build/release/_deps/geos-src/src/deps -isystem /__w/presto/presto/presto-native-execution/_build/release/_deps/geos-build/src/deps -mavx2 -mfma -mavx -mf16c -mlzcnt -mbmi2 -Wno-nullability-completeness -Wno-deprecated-declarations -Wreorder -mavx2 -mfma -mavx -mf16c -mlzcnt -mbmi2 -D USE_VELOX_COMMON_BASE -D HAS_UNCAUGHT_EXCEPTIONS -DFOLLY_CFG_NO_COROUTINES -Wall -Wextra -Wno-unused -Wno-unused-parameter -Wno-sign-compare -Wno-ignored-qualifiers -Wno-implicit-fallthrough -Wno-class-memaccess -Wno-comment -Wno-int-in-bool-context -Wno-redundant-move -Wno-array-bounds -Wno-maybe-uninitialized -Wno-unused-result -Wno-format-overflow -Wno-strict-aliasing -Wno-restrict -Werror -O3 -DNDEBUG -std=gnu++20 -fPIC -Wno-deprecated-declarations -ffp-contract=off -MD -MT

@czentgr, @majetideepak : Is this because of ccache use ?

@MBkkt

I see facebookincubator/velox#15322. Will advance further.

@aditi-pandit
Copy link
Contributor Author

aditi-pandit commented Oct 31, 2025

Error for macos build still remains:

ld: Undefined symbols:
  _AbslInternalSpinLockWake_lts_20250814, referenced from:
      absl::lts_20250814::base_internal::SpinLockWake(std::__1::atomic<unsigned int>*, bool) in libvelox_functions_prestosql.a[10](DateTimeFunctionsRegistration.cpp.o)
  absl::lts_20250814::base_internal::SpinLockWait(std::__1::atomic<unsigned int>*, int, absl::lts_20250814::base_internal::SpinLockWaitTransition const*, absl::lts_20250814::base_internal::SchedulingMode), referenced from:
      void absl::lts_20250814::base_internal::CallOnceImpl<void (*)(re2::LazyRE2 const*), re2::LazyRE2 const*>(std::__1::atomic<unsigned int>*, absl::lts_20250814::base_internal::SchedulingMode, void (*&&)(re2::LazyRE2 const*), re2::LazyRE2 const*&&) in libvelox_functions_prestosql.a[10](DateTimeFunctionsRegistration.cpp.o)
  absl::lts_20250814::raw_log_internal::RawLog(absl::lts_20250814::LogSeverity, char const*, int, char const*, ...), referenced from:
      void absl::lts_20250814::base_internal::CallOnceImpl<void (*)(re2::LazyRE2 const*), re2::LazyRE2 const*>(std::__1::atomic<unsigned int>*, absl::lts_20250814::base_internal::SchedulingMode, void (*&&)(re2::LazyRE2 const*), re2::LazyRE2 const*&&) in libvelox_functions_prestosql.a[10](DateTimeFunctionsRegistration.cpp.o)
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@czentgr @majetideepak : I am seeing this error only on this build. The history for the CI job is green as well. Can't pinpoint from the Velox changes what might have caused this. Any suggestions ?

amitkdutta
amitkdutta previously approved these changes Oct 31, 2025
Copy link
Contributor

@amitkdutta amitkdutta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aditi-pandit

Copy link
Contributor

@czentgr czentgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MBkkt
Copy link
Contributor

MBkkt commented Nov 1, 2025

@aditi-pandit Hi, did fast-float error fixed? We had separate PR for this facebookincubator/velox#15322

@MBkkt
Copy link
Contributor

MBkkt commented Nov 1, 2025

@aditi-pandit

I am seeing this error only on this build. The history for this is green as well. Can't pinpoint from the Velox changes what might have caused this. Any suggestions ?

I think this also can be related to my commit with usage LazyRE2 (see compiler error trace)
I think it was just incorrect build code that wasn't checked before recent changes

@aditi-pandit aditi-pandit merged commit cc966ee into prestodb:master Nov 1, 2025
80 of 81 checks passed
@aditi-pandit
Copy link
Contributor Author

@aditi-pandit Hi, did fast-float error fixed? We had separate PR for this facebookincubator/velox#15322

Thanks @MBkkt. Yes, that PR helped. I moved the Velox update post that commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:IBM PR from IBM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants